box: box.New returns an error instead of panic #473
Merged
+80
−38
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
oleg-jukovec
requested changes
Oct 23, 2025
Collaborator
oleg-jukovec
left a comment
oleg-jukovec
left a comment
There was a problem hiding this comment.
Thank you for the patch. See comments below.
Please, add an entry into the migration guide about the breaking change:
Line 7 in 6111b79
oleg-jukovec
requested changes
Oct 23, 2025
Collaborator
There was a problem hiding this comment.
A commit message should be well-formatted too:
50 chars max for a title line.
72 chars max for a message line.
box: box.New returns an error instead of panic
Method `box.New` now returns (*Box, error), because avoiding panics
entirely in third-party libraries is ideal, since we can't break
compatibility
Nonetheless, there is still a way to panic on error, via implemented
`box.MustNew` wrapper.
Closes #448
See:
laines-it
commented
Oct 25, 2025
Collaborator
Author
There was a problem hiding this comment.
Commit message is bad-formatted. Should be:
Tests now use `box.MustNew` instead of `box.New`.
Added TestMocked_BoxNew for box.New creating a workable object
by means of checking length of Request[] in mockDoer after subrequest.
Closes #448
oleg-jukovec
requested changes
Oct 27, 2025
Collaborator
oleg-jukovec
left a comment
oleg-jukovec
left a comment
There was a problem hiding this comment.
Please, rewrite commit messages according the rules:
<=50 char for a title line
<= 72 chars for a message line
laines-it
force-pushed
the
laines-it/gh-448-box-New-return-error
branch
from
b031345 to
d839e4c
Compare
oleg-jukovec
requested changes
Oct 27, 2025
Collaborator
There was a problem hiding this comment.
Please, format the first commit correctly according to the rules:
box: box.New returns an error instead of panic
Method `box.New` now returns (*Box, error), because avoiding panics entirely in third-party libraries is ideal, since we can't break compatibility
Nonetheless, there is still a way to panic on error, via implemented `box.MustNew` wrapper.
Closes #448
->
box: box.New returns an error instead of panic
Method `box.New` now returns (*Box, error), because avoiding panics
entirely in third-party libraries is ideal, since we can't break
compatibility.
Nonetheless, there is still a way to panic on error, via implemented
`box.MustNew` wrapper.
Part of #448
And it should be Part of, not Closes since it is a not last commit in a sequence related to the issue.
Method `box.New` now returns (*Box, error), because avoiding panics entirely in third-party libraries is ideal, since we can't break compatibility. Nonetheless, there is still a way to panic on error, via implemented `box.MustNew` wrapper. Part of #448
Tests now use `box.MustNew` instead of `box.New`. Added TestMocked_BoxNew for box.New creating a workable object by means of checking length of Request[] in mockDoer after subrequest. Closes #448
laines-it
force-pushed
the
laines-it/gh-448-box-New-return-error
branch
from
d839e4c to
5490645
Compare
oleg-jukovec
approved these changes
Oct 27, 2025
Collaborator
oleg-jukovec
left a comment
oleg-jukovec
left a comment
There was a problem hiding this comment.
Thank you for the patch!
bigbes
approved these changes
Oct 27, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Method
box.Newnow returns(*Box, error), because avoiding panics entirely in third-party libraries is ideal, since we can't break compatibility.Nonetheless, there is still a way to panic on error, via implemented
box.MustNewwrapper.I didn't forget about (remove if it is not applicable):
Closes #448